Skip to content
This repository has been archived by the owner on Feb 14, 2024. It is now read-only.

test: Add new test skeleton based on Terratest and add/extend tests for multiple modules #257

Merged
merged 35 commits into from
Feb 2, 2023
Merged

test: Add new test skeleton based on Terratest and add/extend tests for multiple modules #257

merged 35 commits into from
Feb 2, 2023

Conversation

sebastianczech
Copy link
Contributor

@sebastianczech sebastianczech commented Jan 9, 2023 •
edited
Loading

Description

PR delivers:

  • new test skeleton based on Terratest
  • tests for many modules, in which new test skeleton was used (check list below what issues for which modules were resolved)

Motivation and Context

PR solves:

How Has This Been Tested?

Code was tested by running Terratest as described in README.md in tests folder.

Screenshots (if appropriate)

Not appropriate

Types of changes

  • New feature (non-breaking change which adds functionality)

Checklist

  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes if appropriate.
  • All new and existing tests passed.

@sebastianczech sebastianczech changed the title Test skeleton 165 chore: Add new test skeleton based on Terratest Jan 9, 2023
@sebastianczech sebastianczech marked this pull request as ready for review January 9, 2023 08:41
@sebastianczech sebastianczech requested a review from a team as a code owner January 9, 2023 08:41
pimielowski
pimielowski previously approved these changes Jan 9, 2023
View reviewed changes
Copy link
Contributor

@pimielowski pimielowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍 :)

@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest chore: Add new test skeleton based on Terratest, extend tests for module vpc_route Jan 9, 2023
@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest, extend tests for module vpc_route chore: Add new test skeleton based on Terratest, extend tests for module vpc_route and bootstrap Jan 10, 2023
@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest, extend tests for module vpc_route and bootstrap chore: Add new test skeleton based on Terratest, tests for module vpc_route, bootstrap, panorama Jan 11, 2023
@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest, tests for module vpc_route, bootstrap, panorama chore: Add new test skeleton based on Terratest and add/extend tests for many modules Jan 13, 2023
lstadnik
lstadnik previously approved these changes Jan 18, 2023
View reviewed changes
Copy link
Contributor

@lstadnik lstadnik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great Job! - general comment - when the terratest example are prepared, use as small number resources as possible - for instance - if there is not testing panorama features related with bootstrap - do not create s3 with bootstrap in such example. It extends the testing effort, costs etc.

@sebastianczech
Copy link
Contributor Author

Great Job! - general comment - when the terratest example are prepared, use as small number resources as possible - for instance - if there is not testing panorama features related with bootstrap - do not create s3 with bootstrap in such example. It extends the testing effort, costs etc.

Thanks for feedback @lstadnik . I will review all tests, which we prepared and think how we can optimise it. Regarding bootstrap module and S3 - in order to verify scenario with creating IAM role or other scenario with not creating, but reusing existing IAM role, I needed to create S3 bucket. In your opinion we shouldn't create S3 bucket, because we are not testing it with Panorama ?

@sebastianczech sebastianczech requested a review from migara January 18, 2023 17:55
@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest and add/extend tests for many modules chore: Add new test skeleton based on Terratest and add/extend tests for many modules, change KMS key ID in panorama module Jan 18, 2023
@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest and add/extend tests for many modules, change KMS key ID in panorama module chore: Add new test skeleton based on Terratest and add/extend tests for many modules, change KMS key ID in panorama example Jan 19, 2023
@sebastianczech
Copy link
Contributor Author

sebastianczech commented Jan 19, 2023 •
edited
Loading

After consulting with @pimielowski problem with KMS ID, introduced changes in panorama module were restored and standalone_panorama example was fixed as we need only to pass target_key_arn for KMS key id, not ARN of the key alias.

@sebastianczech sebastianczech dismissed stale reviews from pimielowski and lstadnik January 19, 2023 12:57

From your approval there were a lot of changes. Could you review it one more time, please?

@sebastianczech sebastianczech mentioned this pull request Jan 24, 2023
Merged
4 tasks
@sebastianczech sebastianczech changed the title chore: Add new test skeleton based on Terratest and add/extend tests for many modules, change KMS key ID in panorama example test: Add new test skeleton based on Terratest and add/extend tests for many modules, change KMS key ID in panorama example Jan 24, 2023
@sebastianczech
Copy link
Contributor Author

As @migara proposed, changes in modules and fixes in example were moved to #279

@sebastianczech sebastianczech changed the title test: Add new test skeleton based on Terratest and add/extend tests for many modules, change KMS key ID in panorama example test: Add new test skeleton based on Terratest and add/extend tests for multiple modules Jan 25, 2023
@sebastianczech sebastianczech merged commit fdb85b3 into main Feb 2, 2023
@sebastianczech sebastianczech deleted the test-skeleton-165 branch February 2, 2023 07:08
@github-actions
Copy link
Contributor

🎉 This PR is included in version 0.4.2 🎉

The release is available on Terraform Registry and GitHub release

Posted by semantic-release bot

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@migara migara migara approved these changes

@lstadnik lstadnik Awaiting requested review from lstadnik

@pimielowski pimielowski Awaiting requested review from pimielowski

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@sebastianczech @pimielowski @migara @lstadnik